Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
审核中发现两个错误分类边界还需要处理： 1. 真正的程序错误可能被前面的诊断掩盖如果渲染过程中先记录了一个布局问题，随后代码本身又发生意外异常，当前 JSON receipt 只会显示前面的布局问题，看不到真正导致程序失败的内部错误。 这样用户可能会一直修改图表，但实际上需要修复的是程序代码。 建议调整为：只有最终抛出的异常本身带有明确诊断时，才使用已记录的诊断；否则应报告 2. 部分程序错误会被误报成“输出路径不可写”现在只要异常带有字符串形式的 例如 建议只把确定来自文件系统操作的异常归类为 非阻塞建议
这项不阻塞当前修复。如果不方便抽成共享模块，建议至少增加一致性测试，同时继续保证 本地聚焦回归 31/31 通过，GitHub 最终头提交的 Node 18/20/22/24、三平台包冒烟和 ZIP 新鲜度检查也均通过；上面两个问题属于当前测试没有覆盖到的异常组合。 |
|
Hi @tt-a1i — following up on the workflow library proposal in #222 to coordinate the diagnostic entrypoint. At Would you prefer moving installation into I have a private experiment on |
|
Hi @tt-a1i — #418 is now synchronized with main I propose landing #418 first, then updating #396 on top. The import-side-effect fix is independently useful and does not change this PR's error classifications. When updating #396, please preserve boundary installation at the start of The earlier local rehearsal against your unchanged head Does this order work for you, or would you prefer #396 first? If #396 should land first, I can adapt #418 afterward, including the explicit probe initialization. #418 remains Draft pending that coordination. |
3 participants
Problem and value
On main
6db72a9,node archify/bin/archify.mjs render architecture /no/such.json /tmp/diagram.htmlexits 1 with a raw Node stack. A valid-schema diagram with an overwide node label does the same, even though its renderer already attaches a layout diagnostic. A failed output write can also be reported asinput/readby the JSON boundary because it guesses the operation fromEISDIR/EACCES.Classify read, JSON parse, and output filesystem failures where those operations occur. Human renderer failures now format attached diagnostics with their existing rule codes and supported repairs; an unexpected implementation exception retains its stack and exit 1. This is a narrow defect with a concrete reproduction, so it needs no separate planning issue.
Stability impact
renderand direct renderer scripts are covered.validate --json/deliver --jsonreceipt structure, and standalonedoctorstartup. The public CLI source is unchanged. No new flag, schema, geometry, or Viewer change.input/readoroutput/writeat the actual operation, including parent-directory creation. JSON parse errors are classified only aroundJSON.parse. Existing classified output-path errors keep their own code. Unexpected implementation failures are not guessed to be malformed JSON or unreadable input.renderwrites.deliverremains the interface that preserves the prior artifact until verification and commit succeed.Tests run
Base
6db72a9aea3d0f67a6a034e41f8a5491476a11c1; current candidate80187faf511740005c8982e4a2514f132e08e40b. Local macOS, official Node 22.23.2. The working tree tested is identical to this commit.node --test archify/test/render-failure-diagnostics.test.mjs archify/test/repair-receipt.test.mjs archify/test/cli-output-types.test.mjs: 34 passed, 0 failed, 0 skipped.writeDiagramreceiving an undefined output path in human/JSON modes. A formatter consistency test covers the non-blocking review suggestion without adding a renderer dependency to standalonedoctor.npm testfromarchify/: 1360 total; 1312 passed, 1 failed, 47 skipped. The only failure ischecked-in MCO artifacts are byte-reproducible from the pinned repository revision; the same focused test fails on clean base6db72a9. This is not a green full-suite claim. Optional/browser skips remain skips. The complete suite includes the standalonedoctorregressions.node_modules; all 15 render-failure tests passed.bfb7603, onlyrenderers/shared/cli.mjsandrenderers/shared/diagnostics.mjschange in the archive.git diff --checkpassed.bfb7603remain historical evidence: this revision changes only failure guards and diagnostic selection, with successful public/direct output contracts retested above.Review follow-up
Addresses the two classification findings from @sunsunsun-java: recorded diagnostics are used only when the final exception has nonempty attached diagnostics; otherwise the receipt reports
internal/unclassified. Read/write classification now requires filesystemcode,syscall, and numericerrno, preserving implementation errors and existing classified path guards. The formatter suggestion is covered by a consistency regression while retaining standalonedoctorstartup.Visual evidence
Not applicable. Only failure presentation/classification changes. The five-mode successful HTML comparison is byte-identical; no rendered appearance, interaction, or geometry is changed.
Generated artifacts
archify.ziprebuilt with official Node 22.23.2. All 79 archive entries are retained; changed entries are onlyreferences/delivery-contract.md,renderers/shared/cli.mjs, andrenderers/shared/diagnostics.mjs. No gallery, examples, or other generated content changed because their rendered bytes remain identical.